Switch most everything to netip in prep for ipv6 in the overlay #1173
Conversation
| "net/netip" | ||
| ) | ||
|
|
||
| type VpnIp uint32 |
There was a problem hiding this comment.
leaving a note to consider bringing this back as type VpnIp netip.addr mostly as documentation, so we don't forget
There was a problem hiding this comment.
or at least type VpnIp=netip.addr
| b := net.ParseIP(a) | ||
| b, err := netip.ParseAddr(a) | ||
| if err != nil { | ||
| return |
| IP: net.ParseIP(sHost), | ||
| Port: port, | ||
| } | ||
| r.outNat[c.GetUDPAddr().String()+":"+fromAddr.String()] = toAddr |
There was a problem hiding this comment.
idk if you wanna mess with it, but I think we have the opportunity to use an AddrPort over a string here
There was a problem hiding this comment.
This is an internal NAT for the e2e tests, both sides are netip.AddrPorts. Could move this to a struct containing two netip.AddrPorts to avoid the strings
| } | ||
|
|
||
| v6 := sa.(*windows.SockaddrInet6) | ||
| return &Addr{ |
There was a problem hiding this comment.
does golang have a htons equivalent?
| } | ||
|
|
||
| // Make sure we don't have any unexpected fields | ||
| assertFields(t, []string{"VpnIp", "LocalIndex", "RemoteIndex", "RemoteAddrs", "Cert", "MessageCounter", "CurrentRemote", "CurrentRelaysToMe", "CurrentRelaysThroughMe"}, thi) | ||
| test.AssertDeepCopyEqual(t, &expectedInfo, thi) | ||
| assert.EqualValues(t, &expectedInfo, thi) | ||
| //TODO: netip.Addr reuses global memory for zone identifiers which breaks our "no reused memory check" here |
There was a problem hiding this comment.
Not sure what to do with this one
There was a problem hiding this comment.
Can we write a function to check everything but the zone?
Or, for the purpose of testing, we could (somehow?) ensure that each netip.Addr we check like this has a unique zone, but that sort of sucks
| for _, ip := range c.Details.Ips { | ||
| remoteCidr.AddCIDR(&net.IPNet{IP: ip.IP, Mask: net.IPMask{255, 255, 255, 255}}, struct{}{}) | ||
| //TODO: IPV6-WORK what to do when ip is invalid? |
There was a problem hiding this comment.
Need to determine failure strategy
| } | ||
|
|
||
| if ip4 := fIp.To4(); ip4 != nil && lh.myVpnNet.Contains(fIp) { | ||
| //TODO: we could technically insert all returned ips instead of just the first one if a dns lookup was used |
There was a problem hiding this comment.
Do we want to include this in the change here?
| @@ -146,12 +146,23 @@ func (u *StdConn) ListenOut(r EncReader, lhf LightHouseHandlerFunc, cache *firew | |||
| //metric.Update(int64(n)) | |||
| for i := 0; i < n; i++ { | |||
| if u.isV4 { | |||
| udpAddr.IP = names[i][4:8] | |||
| ip, _ = netip.AddrFromSlice(names[i][4:8]) | |||
| //TODO: IPV6-WORK what is not ok? | |||
There was a problem hiding this comment.
Need to determine what to do here
nbrownus
changed the title
huch, really? I'm the author of |
Open questions:
netip.AddrPortremote which leads to loggingInvalid AddrPortinstead ofnillike before, how do we want to deal with it?Some things to note:
netip.Addrcreations require the resulting address to beUnmap()d to avoid equality issues between 4in6 and v4 addresses.ffff:127.0.0.1!=127.0.0.1and its easy to get the former.bartappears to be pretty slow when compared with our oldcidr.Tree. We should look at performance carefully and determine if we need to resurrect that code.